Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Options to fix dependencies of manually loaded packages #79

Merged
merged 14 commits into from
Sep 15, 2023

Conversation

joemarshall
Copy link
Contributor

Right now, if you import packages from wheel files without automatically getting dependencies, micropip.freeze won't correctly show dependencies for the packages.

This patch fixes up dependencies for manually loaded wheels, adding a method micropip.check_package_dependencies to check and fix dependencies for a package.

It also uses the same function to make micropip.freeze work even if you just manually load a wheel and all dependencies.

Still not sure whether this would be better in pyodide-lock, but I think given micropip.freeze exists, this patch that makes it work in this case seems reasonably sensible.

Copy link
Member

@hoodmane hoodmane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @joemarshall. I made some nitpicky comments.

@joemarshall
Copy link
Contributor Author

I refactored it so fix_package_dependencies lives in _utils and got rid of the public facing function which wasn't really useful except in conjuction with freeze anyway.

@joemarshall
Copy link
Contributor Author

Oh and fixed / added tests for freeze.

Copy link
Member

@ryanking13 ryanking13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @joemarshall!

I have some minor comments otherwise LGTM.

Could you please update the changelog? I think we can use a version 0.4.1 and release with this PR.

micropip/_utils.py Outdated Show resolved Hide resolved
micropip/_utils.py Outdated Show resolved Hide resolved
@joemarshall
Copy link
Contributor Author

Okay, I think I've done those changes okay now

@ryanking13
Copy link
Member

Thanks @joemarshall! Pyodide 0.24.0 has already been released, so I think this would be included in 0.24.1

@ryanking13 ryanking13 merged commit 2663afe into pyodide:main Sep 15, 2023
7 checks passed
@joemarshall joemarshall deleted the fix_dependencies branch September 15, 2023 09:09
@hoodmane hoodmane added this to the 0.24.1 milestone Sep 15, 2023
@hoodmane
Copy link
Member

Oh wait this is a micropip change, not sure if these labels make sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants